Skip to content

Adding a special case to _common so that Data Ops get a more informative message#1607

Merged
jeromedockes merged 7 commits into
skrub-data:mainfrom
rcap107:dop-better-error-message-dispatch
Sep 19, 2025
Merged

Adding a special case to _common so that Data Ops get a more informative message#1607
jeromedockes merged 7 commits into
skrub-data:mainfrom
rcap107:dop-better-error-message-dispatch

Conversation

@rcap107

@rcap107 rcap107 commented Sep 12, 2025

Copy link
Copy Markdown
Member

I added a check to the dispatch module so that if it gets passed a Data Op a more specific error message is shown.
This should address #1477

With this,

import skrub
import pandas as pd

df = pd.DataFrame({"value": [12,3]})

v = skrub.var("data", df)

skrub.TableReport(v)

gets the message

TypeError: Operation not supported on DataOps. Expecting a Pandas or Polars object, but got a DataOp. You may want to use `.skb.eval()` to evaluate the DataOp and obtain a dataframe.

@rcap107 rcap107 added the data_ops Something related to the skrub DataOps label Sep 12, 2025
@rcap107 rcap107 marked this pull request as ready for review September 12, 2025 14:49
@rcap107 rcap107 added this to the 0.6.2 milestone Sep 12, 2025
GaelVaroquaux
GaelVaroquaux reviewed Sep 15, 2025
View reviewed changes
Comment thread skrub/_dataframe/_common.py Outdated
GaelVaroquaux
GaelVaroquaux reviewed Sep 15, 2025
View reviewed changes
Comment thread skrub/_dataframe/_common.py Outdated
GaelVaroquaux
GaelVaroquaux reviewed Sep 15, 2025
View reviewed changes
Comment thread skrub/_dataframe/tests/test_common.py Outdated
dop = [skrub.var("a")] * n_params
with pytest.raises(
TypeError,
match=r"Operation not supported on DataOps.*Expecting a Pandas*",

@GaelVaroquaux GaelVaroquaux Sep 15, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to change this :)

rcap107 and others added 3 commits September 16, 2025 16:30
@rcap107 @GaelVaroquaux
Apply suggestions from code review
db659eb 
Co-authored-by: Gael Varoquaux <gael.varoquaux@normalesup.org>
@rcap107
fixing test
d6f55a5
@rcap107
reworking message
e0ff048
jeromedockes
jeromedockes reviewed Sep 17, 2025
View reviewed changes
Comment thread skrub/_dataframe/_common.py
@rcap107 rcap107 mentioned this pull request Sep 18, 2025
Closed
jeromedockes
jeromedockes approved these changes Sep 19, 2025
View reviewed changes

@jeromedockes jeromedockes left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you @rcap107 !

@jeromedockes jeromedockes merged commit 9f793ba into skrub-data:main Sep 19, 2025
26 checks passed
@jeromedockes jeromedockes mentioned this pull request Sep 19, 2025
Closed
rcap107 added a commit to rcap107/skrub that referenced this pull request Sep 25, 2025
@rcap107
Adding a special case to _common so that Data Ops get a more informat…
43c0cdb 
…ive message (skrub-data#1607)
rcap107 added a commit that referenced this pull request Sep 25, 2025
@rcap107
Adding a special case to _common so that Data Ops get a more informat…
a485f13 
…ive message (#1607)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@GaelVaroquaux GaelVaroquaux GaelVaroquaux left review comments
@jeromedockes jeromedockes jeromedockes approved these changes

Assignees

No one assigned

Labels

data_ops Something related to the skrub DataOps

Projects

None yet

Milestone

0.6.2

Development

Successfully merging this pull request may close these issues.

3 participants

@rcap107 @GaelVaroquaux @jeromedockes